Restore bottom positioning for ordinary routed threads - #72

Carl, an automated reviewer, commenting via Wes’s GitHub account.

No blocking findings

Reviewed head 8b9082ef15dd716fbfecff0d0101272703271b29 against base 7ce898eb6f77ca4fb95f06a7c7789f63245dee28.

The production change stays within the existing thread presentation owner: ordinary root routes no longer invoke exact-message reveal, acknowledgement clears the navigation deadline without settling scrolling, and bounded pagination still controls initial bottom positioning unless reader intent takes over. Exact-target reveal/focus, request revocation, and history routing retain their existing paths.

Reviewed the complete diff and the owning UI, navigation controller/presentation lifetimes, thread reader, and reading hooks. Regression coverage includes routed/non-routed bounded loading, reader gestures, one-time completion, cancellation, a real-controller 16-second deadline case, and held-pagination browser journeys with live updates. git diff --check passed. No local test suites were rerun; existing CI supplies broad validation.

CI snapshot for this head: JavaScript (including full Vitest), Rust/tool integration, Windows notification checks, browser measurements, and DCO passed. Chromium’s complete message-navigation.spec.mjs and navigation-thread-history.spec.mjs files passed (15 cases). Both engines’ first journey shard failed at composer-links.spec.mjs:31 (expected two GitHub-decorated composer links, got zero); the WebKit second shard was still running. That failing standalone composer fixture does not mount ThreadPanel or use the modified browser fixture, so I found no change-related cause in this diff. Treat it as a separate unresolved CI gate, not a passing build or a proven flake.

No local runtime, live-relay, native-GUI, or screen-reader certification. Existing ten-page oldest-first history limits remain unchanged: bottom means bottom of returned history, not necessarily the newest reply.

No approval or merge action submitted.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source review clear

Reviewed 05b873e against exact base/merge-base 7ce898e. No actionable code finding. The independent fixture/selector lane returned clear and was verified against the owning source.

• Ordinary root visits acknowledge the mounted thread before the navigation deadline without settling scrolling. Bounded history still controls bottom positioning unless reader intent intervenes. Exact-message reveal/focus, retained anchors, Back/Forward, and revoked presentation retain their existing owners. The production and owner-test blobs are unchanged from the previously reviewed 8b9082e.
• Reviewed both new test corrections against their actual consumers: the link-composer preview supplies the stable empty typing store required by TypingIndicator; the Back test selects the real Beta channel button and explicitly waits for its unread badge. Assertions, deadlines and retry settings are not relaxed.
• Exact-head hosted CI is green: run 34959748245 passed all nine CI jobs, including browser measurements and all four Chromium/WebKit journey shards. DCO, Semgrep and zizmor also passed. Exact-object git diff --check passed.

Source/metadata inspection only on the authorized laptop; no checkout, dependency installation, builds, tests or PR-code execution by this automation. No live-relay, native-GUI or screen-reader certification. The existing ten-page oldest-first limit remains: bottom means bottom of returned history, not necessarily the newest reply. This is a COMMENT, not approval or a merge action.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Updated on current main; no remaining code findings

I integrated main 42984601ebe1327006526cb03d5335264b377537 into this branch without rewriting the original commits. New head: 9ef87573ec5fca2967fab8db398e47bcc0dd38be. This is an implementation/validation record, not independent human approval.

The three test conflict resolutions retain main’s media coverage, deterministic unread/head barriers, reduced fixture histories and link-composer styling/typing contract. The production patch remains +19/-6 in ThreadPanel.tsx. Ordinary root navigation acknowledges presentation before the deadline; bounded history and reader intent still own positioning. Exact reveal, cancellation, session replacement, Back, and focus paths retain their existing owners. Princess Donut independently reviewed this exact integrated head with no actionable findings; I verified the resolution and evidence.

Clean exact-head validation: mandatory hooks passed formatting/lint, TypeScript, 47 related tests in seven files (all 31 ThreadPanel cases included), and design types/guards. The complete navigation-thread-history, message-navigation, and composer-links browser files passed 32/32 in Chromium + WebKit, zero retries, 57.8s. Diff checks and all commit sign-off trailers passed. Hosted DCO passed. No assertions, timeouts, or tolerances were relaxed.

Remaining merge gates: current-head broad CI (run 35644167735, running at the latest inspection) and required reviewer/code-owner approval. GitHub reports MERGEABLE. Old-head green CI is not credited to this merge. No live-relay/native-GUI/screen-reader or release certification; existing oldest-first ten-page limits remain. No approval or merge action submitted.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Source re-review clear; CI is not green

Reviewed head 9ef8757 against exact base 4298460. No actionable PR-introduced code findings. This is COMMENTED, not approval or merge-readiness certification.

Integration and contract

The production patch remains +19/-6 in ThreadPanel.tsx. I compared it with the previously reviewed patch and traced the current owning functions and callers, rather than treating a matching patch as sufficient integration evidence.

• Ordinary root navigation acknowledges its mounted root before the navigation deadline, without invoking exact-root reveal or settling reading. Bounded returned history still owns initial bottom positioning, unless reader input intervenes. Exact-message reveal/focus, request revocation, Back/Forward, read observation and retained anchors keep their distinct owners.
• Current-main media review, Sessions and agent/design-system integration are retained. The merged test helper preserves navigation and media argument positions; main’s deterministic unread/head barriers and reduced channel histories remain. The two held-pagination geometry cases and opt-in no-delivery reply seeding retain their assertions and isolation.
• The ten-page oldest-first bound remains intentional: bottom is the bottom of returned history, not a guarantee of the newest reply. Flat Sessions and exact top-level timeline targets retain their separate presentation owners.

Independent challenge disposition

The independent reviewer verified the merge resolution and all combined helper callers. I checked its three additional concerns against the agreed contract and prior reviewed behavior:

• Ordinary opening intentionally does not force focus into a message row. Read observation still requires actual history focus, visibility and dwell; an opened visit alone is not a read. The Close-button focus assertion preserves that distinction.
• A mounted cached root may acknowledge presentation before the history read finishes. A later history failure remains visible with retry; it does not retroactively turn a presented visit into navigation failure. Pre-presentation errors still fail the pending attempt. Waiting for complete history would undo the deadline/positioning separation this patch exists to provide.
• The exact wheel-displacement test concern is unproven robustness hardening, not the hosted failure below or a new merge regression. No assertions, tolerances or retries were relaxed.

These do not justify reopening the already-reviewed behavior or expanding this merge refresh. No source lane remains outstanding.

Existing exact-head CI failure, not waived

Run 35644167735 completed with both browser 1/2 shards and CI required failed. The other nine reported checks passed. Chromium and WebKit each failed layout.spec.mjs:636, in “panel restoration yields to a new wheel reading position” (107 passed / 1 failed per job).

I traced that failure to its actual owner: the test opens Bestie beside ChannelTimeline, never ThreadPanel. The test, timeline helper and channel scroll owner are byte-identical to the pinned base. This PR’s shared-fixture change only adds optional delivery to app.reply, which that test does not call.

The test observed a decrease in scrollTop, then rejected an unchanged visible message ID. Its tall-row anchor also has a Y coordinate, but the reported assertion ignores Y; equal IDs alone cannot distinguish movement within a tall message from a later snap-back. This establishes a real unresolved hosted failure, not a demonstrated PR-introduced thread regression or a proven flake. Resolve or explicitly disposition the failed gate before merge readiness; this review does not waive it or authorize unrelated repairs.

Evidence limits

Pinned-object source review on Blox only. No checkout, dependency installation, build, test execution, CI rerun, or live UI validation. Base-to-head git diff --check passed. Earlier implementation/local browser results remain separately attributed evidence, not tests run by this automation. No live-relay, native-GUI, screen-reader or release certification.